Repository navigation
EthiopianDateParser: fix capture-group arity mismatch in dateRegex2/3 - #850
ManasBagul23 wants to merge 1 commit into
Conversation
dateRegex2 and dateRegex3 each declared 4 capturing groups (month/day
plus a redundant extra pair of parens wrapped around the \b...\b day
boundary: `(\b(0?[1-9]|[12][0-9]|3[01])\b)`), but catchGeezDate
destructures each match into exactly 3 variables:
for (dateRegex2(month, day, year) <- List(dateString)) ...
for (dateRegex3(day, month, year) <- List(dateString)) ...
Regex.unapplySeq requires the extractor pattern's arity to match the
regex's group count exactly, so a 4-group regex bound to a 3-variable
pattern never matches anything -- these two regexes (of the five total)
have been dead code since they were written.
Fix: drop the outer, redundant capturing parens around \b(...)\b, which
were never used for anything (the day value is already captured by the
inner group) -- \b is a zero-width boundary assertion and does not need
its own group. This brings both regexes down to 3 groups, matching the
existing destructuring pattern, without changing what either regex
actually matches.
Verified with a standalone Java regex check mirroring the exact pattern
structure (java.util.regex has the same group-counting semantics
scala.util.matching.Regex wraps): before the fix, Pattern.compile(...).
groupCount() reports 4 for both; after, 3. Confirmed the patterns still
match their intended date shapes (`<month> <day> <year>` and `<day>
<month> <year>`) unchanged. No Scala/sbt toolchain available locally to
run the project's own test suite.
Related to dbpedia#845 (item C)
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe Ethiopian date parser removes nested capture groups from two date regexes. The resulting captures match the month/day/year and day/month/year extraction order used by ChangesEthiopian date parsing
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~5 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to No actionable merge-blocking risk is established in this change. The relevant tests were inspected but not run, so normal project checks should still be completed. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🧪 Generate unit tests (beta)
🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
Same situation as #849 — PR #846 (merged as c870f5d) already fixes this exact arity mismatch (dateRegex2/3 switched to non-capturing |
Fixes #851 (split out from #845, item C).
dateRegex2anddateRegex3each declared 4 capturing groups (month/day plus a redundant extra pair of parens wrapped around the\b...\bday boundary:(\b(0?[1-9]|[12][0-9]|3[01])\b)), butcatchGeezDatedestructures each match into exactly 3 variables:Regex.unapplySeqrequires the extractor pattern's arity to match the regex's group count exactly, so a 4-group regex bound to a 3-variable pattern never matches anything — these two regexes (of the five total) have been dead code since they were written.Fix: drop the outer, redundant capturing parens around
\b(...)\b, which were never used for anything (the day value is already captured by the inner group) —\bis a zero-width boundary assertion and does not need its own group. This brings both regexes down to 3 groups, matching the existing destructuring pattern, without changing what either regex actually matches.Verification: standalone Java regex check mirroring the exact pattern structure (
java.util.regexhas the same group-counting semanticsscala.util.matching.Regexwraps): before the fix,Pattern.compile(...).groupCount()reports 4 for both; after, 3. Confirmed the patterns still match their intended date shapes (<month> <day> <year>and<day> <month> <year>) unchanged. No Scala/sbt toolchain available locally to run the project's own test suite.Prepared with AI assistance; I reviewed the changes.